-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[Dependency Scanning] Add tracking of the number of dependency queries and emit them as remarks #86079
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@swift-ci test |
|
|
||
| // CHECK-REMARK-SAVE: remark: Number of Swift module queries: '21' | ||
| // CHECK-REMARK-SAVE: remark: Number of named Clang module queries: '6' | ||
| // CHECK-REMARK-SAVE: remark: Number of recorded Clang module dependencies queried by-name from a Swift client: '6' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although it may be prudent to, one day, make the compiler assert/error if the number of queries exceeds the number of recorded by-name Clang dependencies, we are not quite there yet. I have a couple of follow-up changes in the pipeline which should remove further sources of duplicate queries.
|
I guess you forgot to add the Options.td change. I assume I suggested remarks. The other option is to using |
7fed579 to
1349167
Compare
I think remarks fit okay. I have long wanted the scanner to emit more-detailed overall remarks about the work it is doing and putting in all the plumbing this PR adds will make that easier to build on. |
|
@swift-ci test |
| std::shared_ptr<llvm::cas::ActionCache> ActionCache; | ||
|
|
||
| // Metrics about actions performed by the scanner | ||
| std::shared_ptr<ScannerMetrics> scanMetrics; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why this needs to be shared_ptr and same for all other ScannerMetrics references. CAS is shared_ptr mainly because of the APIs from llvm/clang need that. And here it can always just be a reference to ScannerMetrics in ModuleDependencyScanner, or maybe even move to DependencyScannerDiagnosticReporter (where it can decide if it needs to increment the counter and report the metrics based on the flag).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine, I moved the metrics into DependencyScannerDiagnosticReporter and added methods to it which will increment the appropriate counter if remark emission is enabled.
5e76a2f to
bff1749
Compare
|
@swift-ci test |
bff1749 to
eca8925
Compare
|
@swift-ci smoke test |
eca8925 to
e6761b1
Compare
|
@swift-ci smoke test |
|
@swift-ci smoke test Linux platform |
…s and emit them as remarks This change adds collection of three metrics to the scanner: - number of Swift module lookups - number of named Clang module lookups - recorded number of Clang modules which were imported into a Swift module by name It introduces '-Rdependency-scan', which acts as a super-set flag to the existing '-Rdependency-scan-cache' and adds emission of the above metrics as remarks when this flag is enabled. Followup changes will add further remarks about dependency scanner progress.
e6761b1 to
7017034
Compare
|
@swift-ci smoke test |
This change adds collection of three metrics to the scanner:
It re-uses the prior '-Rdependency-scan-cache', renaming it to '-Rdependency-scan' and adds emission of the above metrics as remarks when this flag is enabled. Followup changes will add further remarks about dependency scanner progress.